chore(repositories): Switch over queries for SeerProjectRepository to use ProjectRepository#115456
chore(repositories): Switch over queries for SeerProjectRepository to use ProjectRepository#115456wedamija wants to merge 3 commits into
SeerProjectRepository to use ProjectRepository#115456Conversation
…to use `ProjectRepository` This adds a feature flag that gates switching queries to start using the new `ProjectRepository` table.
github-actions
Bot
added
the
Scope: Backend
…transfer_to cleanup - Remove duplicate ModelRelation(ProjectRepository) in RepositoryDeletionTask.get_child_relations since the shared _get_repository_child_relations helper already includes it. - Add ProjectRepository cleanup to Project.transfer_to so project-repo links are removed when a project moves to a different org.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| repository_id__in=repo_ids, | ||
| project__organization_id=organization_id, | ||
| ).delete() | ||
| org = Organization.objects.get(id=organization_id) |
There was a problem hiding this comment.
Organization.objects.get() raises DoesNotExist and aborts integration cleanup transaction
Line 247 introduces an unguarded Organization.objects.get(id=organization_id) solely to evaluate a feature flag. If the organization has been deleted or is mid-deletion when this RPC runs (e.g., during cascading integration cleanup), it raises Organization.DoesNotExist, which rolls back the surrounding transaction.atomic block and skips the remaining cleanup of ProjectCodeOwners, RepositoryProjectPathConfig, ProjectRepository, and ProjectOption. This leaves orphaned integration data behind.
Verification
Read the full disassociate_organization_integration method (lines 229-291). Confirmed (1) the method is wrapped in transaction.atomic, so a DoesNotExist exception bubbles up and rolls back unrelated cleanup work performed later in the same block; (2) organization_id originates from RPC caller input with no prior validation in this function; (3) features.has() accepts an Organization object — using Organization.objects.filter(id=organization_id).first() with a None fallback would still allow flag evaluation (defaulting to False for unknown orgs) without aborting cleanup. (4) The caller in organizationintegration.py:44-46 explicitly documents this scenario can happen ("This can happen when an organization has been deleted already") and catches CellMappingNotFound, but does not catch Organization.DoesNotExist, confirming this exception will abort the deletion task.
Identified by Warden sentry-backend-bugs · P3Z-ATR
- _write_preferences_to_sentry_db delete: filter by project_repository__project_id and project_repository__repository__status - organization_seer_onboarding_check: filter by project_repository__project__organization_id and status fields - night_shift cron: add select_related for project_repository__project (query filter stays on direct FK since cron is cross-org)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| project_repository__project=project, | ||
| project_repository__repository__status=ObjectStatus.ACTIVE, | ||
| ) | ||
| .select_related("project_repository", "project_repository__repository") |
There was a problem hiding this comment.
N+1 queries from missing select_related("repository") on new path
Medium Severity
When the feature flag is enabled, the new query paths use select_related("project_repository", "project_repository__repository") but the downstream build_repo_definition_from_project_repo function accesses seer_project_repo.repository — the direct FK, not the one traversed through project_repository. Since the direct repository FK is not included in select_related, every iteration triggers a lazy-load DB query, causing an N+1 regression. The old code path correctly used select_related("repository") to eagerly load this relation.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a49b4db. Configure here.
| ModelRelation(Commit, {"repository_id": instance.id}), | ||
| ModelRelation(PullRequest, {"repository_id": instance.id}), | ||
| ModelRelation(RepositoryProjectPathConfig, {"repository_id": instance.id}), | ||
| ModelRelation(ProjectRepository, {"repository_id": instance.id}), |
There was a problem hiding this comment.
Hidden repo cascades into unintended SeerProjectRepository deletion
High Severity
ProjectRepository is added to _get_repository_child_relations, which is shared between RepositoryDeletionTask and repository_cascade_delete_on_hide. The comment on line 37-38 explicitly states that SeerProjectRepository records must NOT be deleted when a repo is merely hidden. However, deleting ProjectRepository during a hide operation triggers ProjectRepositoryDeletionTask, whose child relations include SeerProjectRepository (by project_repository_id), effectively cascade-deleting the very records intended to be preserved. ProjectRepository likely belongs in RepositoryDeletionTask.get_child_relations only, alongside SeerProjectRepository.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a49b4db. Configure here.
| if features.has("organizations:project-repository-fk-reads", project.organization): | ||
| seer_project_repo_qs = ( | ||
| SeerProjectRepository.objects.filter( | ||
| project_repository__project=project, | ||
| project_repository__repository__status=ObjectStatus.ACTIVE, | ||
| ) | ||
| .select_related("project_repository", "project_repository__repository") | ||
| .prefetch_related("branch_overrides") | ||
| ) |
There was a problem hiding this comment.
Bug: The new feature-flagged code path in read_preference_from_sentry_db and bulk_read_preferences_from_sentry_db is missing select_related("repository"), causing an N+1 query issue.
Severity: MEDIUM
Suggested Fix
Add "repository" to the select_related(...) call in the querysets for read_preference_from_sentry_db and bulk_read_preferences_from_sentry_db within the new feature-flagged code path. This will ensure the repository data is prefetched, resolving the N+1 query.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/seer/autofix/utils.py#L659-L667
Potential issue: When the `organizations:project-repository-fk-reads` feature flag is
enabled, the `read_preference_from_sentry_db` and `bulk_read_preferences_from_sentry_db`
functions do not prefetch the `repository` object via `select_related`. However, the
helper function `build_repo_definition_from_project_repo` accesses
`seer_project_repo.repository`, which triggers an additional database query for each
`SeerProjectRepository` being processed. This introduces a classic N+1 query problem in
the new code path, which can lead to significant performance degradation when handling
multiple repositories.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 173ac04. Configure here.
| SeerProjectRepository.objects.filter( | ||
| project_repository__project_id__in=project_ids, | ||
| project_repository__repository__status=ObjectStatus.ACTIVE, | ||
| ).delete() |
There was a problem hiding this comment.
Flag-gated delete misses records with null FK
Medium Severity
When the feature flag is on, the delete query filters through the nullable project_repository FK (project_repository__project_id__in=project_ids). Any SeerProjectRepository rows where project_repository is NULL won't match this join and will survive the delete. The subsequent bulk_create then attempts to insert rows with the same (project, repository) pair, violating the unique_together constraint and raising an IntegrityError that rolls back the entire transaction. No backfill migration was found to ensure project_repository is populated before the flag is enabled.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 173ac04. Configure here.
1 participant
This adds a feature flag that gates switching
SeerProjectRepositoryqueries to start using the newProjectRepositorytable.